Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: Pin pywinpty version to 1.1.0 #101

Merged
merged 10 commits into from
May 18, 2021
Merged

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented May 11, 2021

Fixes #97

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

@jtpio @blink1073, could you please enable the Github Actions workflow here?

@blink1073
Copy link
Contributor

Done, thanks @andfoy!

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

Thanks! @blink1073

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

@blink1073, sorry for pinging again, but it seems that the permission got revoked?

@blink1073
Copy link
Contributor

Ah, probably due to a force push

@blink1073
Copy link
Contributor

We can squash at the end

@blink1073
Copy link
Contributor

Ah, it looks like it isn't here that we are seeing issues, but in the notebook tests

@blink1073
Copy link
Contributor

We only run the most basic test in CI on windows

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

@blink1073 Do the other tests fail here if we try to run them in Windows?

@andfoy andfoy marked this pull request as ready for review May 11, 2021 20:43
@blink1073
Copy link
Contributor

blink1073 commented May 11, 2021

Do the other tests fail here if we try to run them in Windows?

They already did on the older version of pywintpty, which is why we had skipped them originally.

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

Let me try to run them again, and see if they are still failing, maybe they can explain the failures in the notebook

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

This permission setting for first time contributors seems to be very uncomfortable

@blink1073
Copy link
Contributor

I don't see a way to change it in the docs

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

@blink1073, I managed to fix the tests on Windows, Some tests related to single terminals and max terminals hang, so I don't know if that plays part on the notebook errors

@blink1073
Copy link
Contributor

so I don't know if that plays part on the notebook errors

I'm not sure, are you available to test notebook against your pywinpty_1.0.1 branch?

@blink1073
Copy link
Contributor

Thanks for fixing those tests!

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

I'm not sure, are you available to test notebook against your pywinpty_1.0.1 branch?

The RDP session is still open, so I can try to test it

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

I got to reproduce the errors locally, it seems that the errors are related to the terminate method, which I don't know what it does underneath and how it interacts with terminado/pywinpty

@andfoy
Copy link
Member Author

andfoy commented May 11, 2021

I did some changes to ptyprocess in https://github.com/spyder-ide/pywinpty/pull/137/files#diff-4a67b83c782a0c2991144686e69d148dc649326c957c603349b7a3d43f8bab18, however, I don't know if those changes broke something? I don't know if removing the close method on the PTY object is somewhat involved here

@blink1073
Copy link
Contributor

I don't know if those changes broke something

It appears so, because the terminals are successfully terminated if we pin to the previous version.

@andfoy
Copy link
Member Author

andfoy commented May 12, 2021

@blink1073, could you give us a help there? I touched some of the functionality of ptyprocess, but since you wrote the module, maybe you could verify that is working as expected?

@andfoy andfoy changed the title PR: Pin pywinpty version back to 1.0.1 PR: Pin pywinpty version back to 1.1.0 May 17, 2021
@andfoy andfoy changed the title PR: Pin pywinpty version back to 1.1.0 PR: Pin pywinpty version to 1.1.0 May 17, 2021
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic!

@blink1073 blink1073 merged commit fbba87e into jupyter:master May 18, 2021
@stonebig
Copy link

stonebig commented May 22, 2021

hi, is there a possiblity you can produce a wheel for pypy3.7-v7.3.5-win64 ? Is it even possible ? it seems a big challenge to me with rust in the middle not happy with PyPy

@andfoy andfoy deleted the pywinpty_1.0.1 branch May 24, 2021 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pywinpty 1.0 is Incompatible
3 participants